-
Notifications
You must be signed in to change notification settings - Fork 12.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change the hashcounts in raw Lit
variants from usize to u16.
#49993
Conversation
This reduces the size of `Token` from 32 bytes to 24 bytes on 64-bit platforms.
(rust_highfive has picked a reviewer for you, use r? to override) |
Thanks, @nnethercote! Re-assigning to @alexcrichton, since it touches |
@bors: r+ |
📌 Commit 4d34bfd has been approved by |
Out of interest, it feels like this could go lower -- I can't imagine a valid use case for more than 256 of these; did you investigate whether that would net further savings? |
We could get the size of |
Change the hashcounts in raw `Lit` variants from usize to u16. This reduces the size of `Token` from 32 bytes to 24 bytes on 64-bit platforms.
☀️ Test successful - status-appveyor, status-travis |
Oh, btw, the |
I did a rustc-perf run for this. It was a measurable win for some of the coercions and html5ever jobs:
|
The downside is that this makes Literal harder to work with -- you can't pattern match easily the way you can with Option, and creating an empty suffix involves a call to intern(). |
@nnethercote Note that almost nobody should be touching literals though that internal enum, only type-checking and MIR construction. Also, the empty string is preinterned: rust/src/libsyntax_pos/symbol.rs Line 302 in 64e6dda
(although "Invalid" is not the best name for it - cc @petrochenkov @jseyfried) |
The preinterning of the empty string helps; thank you for pointing that out. But overall it's not worth getting rid of the |
This reduces the size of
Token
from 32 bytes to 24 bytes on 64-bitplatforms.